-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sourcery refactored master branch #1
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to GitHub API limits, only the first 60 comments can be shown.
@@ -22,6 +22,7 @@ | |||
Run a Unix socket forkserver. | |||
""" | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 36-40
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
if ignore_case: | ||
attrs = [getattr(source, key) for key in dir(source) | ||
if key.lower() == name.lower()] | ||
|
||
if len(attrs) > 1: | ||
raise ValueError(f"{source.__name__} contains multiple {name} functions.") | ||
elif len(attrs) == 1: | ||
return attrs[0] | ||
else: | ||
return None | ||
else: | ||
if not ignore_case: | ||
return getattr(source, name, None) | ||
attrs = [getattr(source, key) for key in dir(source) | ||
if key.lower() == name.lower()] | ||
|
||
if len(attrs) > 1: | ||
raise ValueError(f"{source.__name__} contains multiple {name} functions.") | ||
elif len(attrs) == 1: | ||
return attrs[0] | ||
else: | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _get_attr
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
if info.defaults is not None: | ||
min_args = max_args - len(info.defaults) | ||
else: | ||
min_args = max_args | ||
|
||
min_args = max_args if info.defaults is None else max_args - len(info.defaults) | ||
if info.varargs is not None: | ||
max_args = math.inf | ||
|
||
ok = (min_args <= max_num_args) and (min_num_args <= max_args) | ||
if not ok: | ||
if min_args == max_args: | ||
args_str = min_args | ||
else: | ||
args_str = f"{min_args}-{max_args}" | ||
args_str = min_args if min_args == max_args else f"{min_args}-{max_args}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function check_num_args
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Replace if statement with if expression [×2] (
assign-if-exp
)
match = address_regex.match(result) | ||
if match: | ||
suspected_address = match.group(2) | ||
if match := address_regex.match(result): | ||
suspected_address = match[2] | ||
# Double check this is the actual address | ||
default_result = object.__repr__(obj) | ||
match2 = address_regex.match(default_result) | ||
if match2: | ||
known_address = match2.group(2) | ||
if match2 := address_regex.match(default_result): | ||
known_address = match2[2] | ||
if known_address == suspected_address: | ||
result = match.group(1) + match.group(3) | ||
result = match[1] + match[3] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _repr_no_address
refactored with the following changes:
- Use named expression to simplify assignment and conditional [×2] (
use-named-expression
) - Replace m.group(x) with m[x] for re.Match objects [×4] (
use-getitem-for-re-match-groups
)
raise ValueError("%s.param_names is not a list of strings" % (name,)) | ||
raise ValueError(f"{name}.param_names is not a list of strings") | ||
|
||
try: | ||
self._params = list(self._params) | ||
except ValueError: | ||
raise ValueError("%s.params is not a list" % (name,)) | ||
raise ValueError(f"{name}.params is not a list") | ||
|
||
if self._params and not isinstance(self._params[0], (tuple, list)): | ||
# Accept a single list for one parameter only | ||
self._params = [self._params] | ||
else: | ||
self._params = [[item for item in entry] for entry in self._params] | ||
self._params = [list(entry) for entry in self._params] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Benchmark.__init__
refactored with the following changes:
- Replace interpolated string formatting with f-string [×2] (
replace-interpolation-with-fstring
) - Replace identity comprehension with call to collection constructor (
identity-comprehension
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
stamp = path + ".timestamp" | ||
stamp = f"{path}.timestamp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function BuildCache._get_cache_dir
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
if hasattr(file, 'isatty'): | ||
return file.isatty() | ||
return False | ||
return file.isatty() if hasattr(file, 'isatty') else False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function isatty
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
try: | ||
with contextlib.suppress(UnicodeError): | ||
fileobj.write(s) | ||
return | ||
except UnicodeError: | ||
pass | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _write_with_fallback
refactored with the following changes:
- Use
contextlib
'ssuppress
method to silence an error (use-contextlib-suppress
)
if i + 1 == len(args): | ||
color = '' | ||
else: | ||
color = args[i + 1] | ||
|
||
color = '' if i + 1 == len(args) else args[i + 1] | ||
if color: | ||
msg = _color_text(msg, color) | ||
_write_with_fallback(msg, file) | ||
|
||
_write_with_fallback(end, file) | ||
else: | ||
for i in range(0, len(args), 2): | ||
msg = args[i] | ||
_write_with_fallback(msg, file) | ||
_write_with_fallback(end, file) | ||
|
||
_write_with_fallback(end, file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function color_print
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
) - Hoist repeated code outside conditional statement (
hoist-statement-from-if
)
if x.strip() == '': | ||
return default | ||
return x | ||
return default if x.strip() == '' else x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_answer_default
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
if len(s) > l: | ||
return '...' + s[-(l - 3):] | ||
else: | ||
return s | ||
return f'...{s[-(l - 3):]}' if len(s) > l else s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function truncate_left
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
if len(parts) == 1: | ||
rest = None | ||
else: | ||
rest = parts[1] | ||
|
||
rest = None if len(parts) == 1 else parts[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Log._stream_formatter
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
) - Simplify conditional into switch-like form [×2] (
switch
) - Move assignments closer to their usage (
move-assign
)
if isatty(sys.stdout): | ||
if time.time() > self._last_dot + 1.0: | ||
color_print('.', 'darkgrey', end='') | ||
sys.stdout.flush() | ||
self._last_dot = time.time() | ||
if isatty(sys.stdout) and time.time() > self._last_dot + 1.0: | ||
color_print('.', 'darkgrey', end='') | ||
sys.stdout.flush() | ||
self._last_dot = time.time() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Log.dot
refactored with the following changes:
- Merge nested if conditions (
merge-nested-ifs
)
for key in platform_keys.keys(): | ||
for key in platform_keys: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function iter_matrix
refactored with the following changes:
- Remove unnecessary call to keys() (
remove-dict-keys
)
# Check if spurious keys left | ||
remaining_keys = tuple(matrix.keys()) | ||
if remaining_keys: | ||
raise util.UserError('Unknown keys in "matrix" configuration: {}, expected: {}'.format( | ||
remaining_keys, matrix_types + tuple(bare_keys))) | ||
if remaining_keys := tuple(matrix.keys()): | ||
raise util.UserError( | ||
f'Unknown keys in "matrix" configuration: {remaining_keys}, expected: {matrix_types + tuple(bare_keys)}' | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _parse_matrix
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Replace call to format with f-string (
use-fstring-for-formatting
)
This removes the following comments ( why? ):
# Check if spurious keys left
filename = os.path.join(html_dir, self.path + ".json") | ||
filename = os.path.join(html_dir, f"{self.path}.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Graph.save
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
if self.scalar_series: | ||
return self._steps[0] | ||
else: | ||
return self._steps | ||
return self._steps[0] if self.scalar_series else self._steps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Graph.get_steps
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
new_steps = [] | ||
|
||
for left, right, cur_val, cur_min, cur_err in steps: | ||
new_steps.append((x[left], x[right - 1] + 1, cur_val, cur_min, cur_err)) | ||
|
||
return new_steps | ||
return [ | ||
(x[left], x[right - 1] + 1, cur_val, cur_min, cur_err) | ||
for left, right, cur_val, cur_min, cur_err in steps | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _compute_graph_steps
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
ys = [[None] * len(x_idx) for j in range(n_series)] | ||
ys = [[None] * len(x_idx) for _ in range(n_series)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _combine_graph_data
refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore
)
path = os.path.join(root, filename) | ||
yield path | ||
yield os.path.join(root, filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function iter_machine_files
refactored with the following changes:
- Inline variable that is immediately yielded (
inline-immediately-yielded-variable
)
if _path is None: | ||
path = cls.get_machine_file_path() | ||
else: | ||
path = _path | ||
|
||
path = cls.get_machine_file_path() if _path is None else _path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function MachineCollection.load
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
if _path is None: | ||
path = cls.get_machine_file_path() | ||
else: | ||
path = _path | ||
if os.path.isfile(path): | ||
d = util.load_json(path) | ||
else: | ||
d = {} | ||
path = cls.get_machine_file_path() if _path is None else _path | ||
d = util.load_json(path) if os.path.isfile(path) else {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function MachineCollection.save
refactored with the following changes:
- Replace if statement with if expression [×2] (
assign-if-exp
)
if _path is None: | ||
path = cls.get_machine_file_path() | ||
else: | ||
path = _path | ||
path = cls.get_machine_file_path() if _path is None else _path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function MachineCollection.update
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
if cls.hardcoded_machine_name: | ||
return cls.hardcoded_machine_name | ||
return _get_unique_machine_name() | ||
return cls.hardcoded_machine_name or _get_unique_machine_name() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Machine.get_unique_machine_name
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Simplify if expression by using or (
or-if-exp-identity
)
d.update(kwargs) | ||
d |= kwargs | ||
if (not len(d) and interactive) or force_interactive: | ||
d.update(self.generate_machine_file(use_defaults=use_defaults)) | ||
d |= self.generate_machine_file(use_defaults=use_defaults) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Machine.load
refactored with the following changes:
- Merge dictionary updates via the union operator [×2] (
dict-assign-update-to-union
)
if m and m.group(1).strip(): | ||
return m.group(1) | ||
if m and m[1].strip(): | ||
return m[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _format_param_value
refactored with the following changes:
- Replace m.group(x) with m[x] for re.Match objects [×2] (
use-getitem-for-re-match-groups
)
if extra_params is None: | ||
extra_params = {} | ||
else: | ||
extra_params = dict(extra_params) | ||
|
||
extra_params = {} if extra_params is None else dict(extra_params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function run_benchmarks
refactored with the following changes:
- Replace if statement with if expression [×2] (
assign-if-exp
)
result = [None for idx in params] | ||
result = [None for _ in params] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function fail_benchmark
refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore
)
if cwd is None: | ||
real_cwd = tempfile.mkdtemp() | ||
else: | ||
real_cwd = cwd | ||
|
||
real_cwd = tempfile.mkdtemp() if cwd is None else cwd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function _run_benchmark_single_param
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
) - Simplify if expression by using or (
or-if-exp-identity
)
env_vars.update(self.env.env_vars) | ||
env_vars |= self.env.env_vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Spawner.create_setup_cache
refactored with the following changes:
- Merge dictionary updates via the union operator (
dict-assign-update-to-union
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.12%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Branch
master
refactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
master
branch, then run:Help us improve this pull request!